Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation read through #219

Open
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

raphaelshirley
Copy link
Member

We are working through installing according to the documentation and making changes as we find them.

Closes #216

Code Quality

  • I have read the Contribution Guide
  • My code follows the code style of this project
  • My code builds (or compiles) cleanly without any errors or warnings
  • My code contains relevant comments and necessary documentation

Project-Specific Pull Request Checklists

Bug Fix Checklist

  • My fix includes a new test that breaks as a result of the bug (if possible)
  • My change includes a breaking change
    • My change includes backwards compatibility and deprecation warnings (if possible)

New Feature Checklist

  • I have added or updated the docstrings associated with my feature using the NumPy docstring format
  • I have updated the tutorial to highlight my new feature (if appropriate)
  • I have added unit/End-to-End (E2E) test cases to cover my new feature
  • My change includes a breaking change
    • My change includes backwards compatibility and deprecation warnings (if possible)

Documentation Change Checklist

Build/CI Change Checklist

  • If required or optional dependencies have changed (including version numbers), I have updated the README to reflect this
  • If this is a new CI setup, I have added the associated badge to the README

Other Change Checklist

  • Any new or updated docstrings use the NumPy docstring format.
  • I have updated the tutorial to highlight my new feature (if appropriate)
  • I have added unit/End-to-End (E2E) test cases to cover any changes
  • My change includes a breaking change
    • My change includes backwards compatibility and deprecation warnings (if possible)

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.93%. Comparing base (0fd8408) to head (7a7075b).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/lephare/data_retrieval.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #219      +/-   ##
==========================================
- Coverage   71.93%   71.93%   -0.01%     
==========================================
  Files          50       50              
  Lines        6432     6435       +3     
  Branches      937      937              
==========================================
+ Hits         4627     4629       +2     
- Misses       1805     1806       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Raphael Shirley and others added 20 commits October 15, 2024 13:44
There was a repeated line with a typo which did not do anything so I removed it.
Following testing by Olivier we realised it was easy to have a notebook kernel that was not
inside the correct environment. We decided that this was a common enough mistake that we should add explicit instructions tot eh documentation
Since including the pybind11 code using the MANIFEST.in file I could install on my mac from source using pip.
I am concerned that this wouldn't work if I did not already have compilers and libraries present so I recommned
people follow the developer install instructions which install the required libraries if there is no binary available
A number of changes suggested in the PR. Some section renames and addition of extra lines to code snippets for
clarity
Some of the change requests were ignored as they are required for the notebook to run.

I have so far left in the onesource examples at the end as they were previously requested. We can discuss if to remove them this week.

The issue with curl to download the para file was that it needed the url updating to the raw file not the html.

The example para file does not run through the downloader because it needs the sed list links to be relative. We can either update the
para file or the downloader to append these links
He requested a couple of changes which will increase run time. We need to check that all notebooks still complete in time for the
documentation. I also added a simple z-z plot
There are various small changes and cleaning things up. We have to set up the environment variables but given the
new structure a user may also need a further environment variable to reference the executables but I will
not mention that for now for simplicity.
Mainly adding git clone code for clarity following Olivier sugestion
I also added a custom css to make the tables wrap so be easier to read

Many changes to keyword descriptions including removing the defunct keywords
Mainly installation instructions
We have moved the developer install to a new section and reordered gettign started
Raphael Shirley and others added 24 commits November 12, 2024 17:38
We wanted to be more informative regarding the contruction of the input tables which have a strict format
simplified and retured the standard filters and removed low level functionality section
We have run through the first main notebook examples and called other developer examples
calzetti.dat not present so changed to SB_calzetti.dat
These are from changes to process. I need to check the notebooks are still working
Also deleted the seperate notebooks page as it is the core of the Python interface page which now has all teh notebooks
When this documentation is merged we will define v0.2.0 and deprecate the GitLab version
hub is requireing a custom user now to prevent getting spammed.

see readthedocs/readthedocs.org#11763
It seems for docs to build we need to add a user but this broke the test so I have removed the test to see if it will tehn build
I had to send a specific downloader to check the call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation read through
1 participant